Skip to content

minor cleanup: fixing or silencing flake8 errors#2731

Merged
henryiii merged 3 commits intopybind:masterfrom
rwgk:test_enum_py_noqa
Dec 19, 2020
Merged

minor cleanup: fixing or silencing flake8 errors#2731
henryiii merged 3 commits intopybind:masterfrom
rwgk:test_enum_py_noqa

Conversation

@rwgk
Copy link
Copy Markdown
Collaborator

@rwgk rwgk commented Dec 18, 2020

Fixing errors that appeared after a flake8 version bump.

@rwgk rwgk requested a review from henryiii December 18, 2020 21:06
Copy link
Copy Markdown
Collaborator

@bstaletic bstaletic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should just disable B015? Either way, approved.

@rwgk
Copy link
Copy Markdown
Collaborator Author

rwgk commented Dec 18, 2020

Thanks @bstaletic ! I'll wait a bit to see if we can get the CI green (waiting for @henryiii's cmake pinning).

I think in general B015 is useful. I feel disabling it completely would be a loss, I'd go there only if we had to put in a whole bunch more of the noqa comments.

Comment thread .github/workflows/ci.yml Outdated
assert m.load_vector_via_caster(v2) == 6
with pytest.raises(TypeError) as excinfo:
cm.load_vector_via_binding(v2) == 6
cm.load_vector_via_binding(v2)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think B015 helps in cases like this. It clarifies the test is really about the call, and not the comparison, which never happens.

@henryiii henryiii merged commit cecdfad into pybind:master Dec 19, 2020
@github-actions github-actions Bot added the needs changelog Possibly needs a changelog entry label Dec 19, 2020
@henryiii henryiii removed the needs changelog Possibly needs a changelog entry label Dec 21, 2020
@rwgk rwgk deleted the test_enum_py_noqa branch January 4, 2021 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants